-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Conversation
Make that 1.6.1 :-) |
@@ -225,11 +225,10 @@ int uv_fs_event_start(uv_fs_event_t* handle, | |||
} | |||
|
|||
if (!handle->buffer) { | |||
handle->buffer = (char*)_aligned_malloc(uv_directory_watcher_buffer_size, | |||
sizeof(DWORD)); | |||
handle->buffer = (char*)uv__malloc(uv_directory_watcher_buffer_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saghul IIUC, here the call to _aligned_malloc
would be replaced (by default) by a call to malloc
, is it intentional and what is the impact of such a change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was reviewed back in the day and Bert pointed out that plain malloc
provided the necessary alignment in that case, that's why we dropped it.
On Jun 11, 2015 21:05, "Julien Gilli" notifications@github.com wrote:
In deps/uv/src/win/fs-event.c
#25475 (comment):@@ -225,11 +225,10 @@ int uv_fs_event_start(uv_fs_event_t* handle,
}if (!handle->buffer) {
- handle->buffer = (char*)_aligned_malloc(uv_directory_watcher_buffer_size,
sizeof(DWORD));
- handle->buffer = (char*)uv__malloc(uv_directory_watcher_buffer_size);
@saghul https://github.com/saghul IIUC, here the call to _aligned_malloc
would be replaced by a call to malloc, is it intentional and what is the
impact of such a change?—
Reply to this email directly or view it on GitHub
https://github.com/joyent/node/pull/25475/files#r32255959.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saghul Sounds good, thank you for the clarification!
@saghul One question just to make sure I don't leave any stone unturned, otherwise that looks good. Will land asap unless the answer to that question means we need to change something. Thank you once again @saghul 👍 |
LGTM. |
@saghul @joyent/node-collaborators Landing with http://jenkins.nodejs.org/job/node-accept-pull-request/127/. |
R=@misterdjules